Skip to content

api: add Config Entries and Discovery Chain endpoints#121

Merged
mbrulatout merged 3 commits into
criteo:masterfrom
mbrulatout:phase2-config-discovery-snapshot
Jul 3, 2026
Merged

api: add Config Entries and Discovery Chain endpoints#121
mbrulatout merged 3 commits into
criteo:masterfrom
mbrulatout:phase2-config-discovery-snapshot

Conversation

@mbrulatout

Copy link
Copy Markdown
Contributor

Summary

Phase 2 of the ongoing API-parity work (follow-up to #120). Adds two more endpoint groups:

  • Config Entries (consul/api/config.py) — generic CRUD for /v1/config, covering all config entry kinds (service-defaults, service-router, service-splitter, service-resolver, proxy-defaults, mesh, ingress-gateway, terminating-gateway, exported-services, api-gateway, etc.) through one Kind/Name-based implementation rather than modeling every kind's schema individually. Includes cas (check-and-set) support on both set() and delete().
  • Discovery Chain (consul/api/discovery_chain.py) — consul.discovery_chain.get(), covering both the plain GET and the POST variant Consul requires when passing override parameters (connect timeout, protocol, mesh gateway mode).

Not included, on purpose:

  • Snapshot — its response is a raw gzip binary blob, but the existing sync/async HTTP clients (consul/std.py, consul/aio.py) unconditionally decode every response body as UTF-8 text. Implementing Snapshot correctly needs a small transport-layer change (an opt-in raw/binary response mode) that's a distinct, separate piece of work deserving its own isolated review rather than being bundled into an "add a CRUD module" PR. Will follow up separately.
  • Peering — deliberately deferred; no current consumer identified for cluster peering.

A bug this surfaced

While testing Config.set()/delete() with cas, I found that PUT /v1/config always returns a literal JSON boolean (true/false) reflecting whether the CAS check passed — but DELETE /v1/config/:kind/:name returns an empty object ({}) when no cas is given, and only returns a real boolean when cas is passed. Using CB.boolean() (which only checks the HTTP status code, not body content) would have silently reported CAS failures as successes. Fixed by using CB.json() for set(), and branching between CB.boolean() / CB.json() in delete() based on whether cas was supplied. Verified against a live Consul instance before writing the test assertions.

Base branch note

This branches off master, not off #120 — the two PRs touch disjoint files except for .pre-commit-config.yaml's mypy fix, which is carried over here too (see last commit) since master doesn't have it yet. ENDPOINT_STATUS.md isn't touched in this PR since it doesn't exist on master yet (it's new in #120); I'll add Config Entries/Discovery Chain rows to it once this rebases past #120's merge.

Test plan

  • mypy consul/ --no-sqlite-cache — clean
  • ruff check . / ruff format --diff . — clean
  • New integration tests in tests/api/test_config.py and tests/api/test_discovery_chain.py, run against real dockerized Consul (1.20.6/1.21.5/1.22.0) — all passing, including both CAS-success and CAS-failure paths on set() and delete()
  • Full existing test suite re-run — no regressions

Mathias Brulatout added 3 commits July 2, 2026 13:01
Adds consul.config for the generic Kind/Name-based config entry
endpoint (PUT/GET/GET-list/DELETE /v1/config), covering all config
entry kinds (service-defaults, service-router, proxy-defaults, mesh,
ingress-gateway, etc.) through one implementation rather than
modeling each kind's schema individually. cas (check-and-set)
support included on both set() and delete().

Note: DELETE /v1/config/:kind/:name returns an empty object when no
cas is given, but a real JSON boolean when cas is passed -- callback
choice (CB.boolean vs CB.json) branches on that.
Adds consul.discovery_chain.get(), covering both the plain GET
(no overrides) and the POST variant Consul requires when passing
OverrideConnectTimeout/OverrideProtocol/OverrideMeshGateway.
master doesn't have this fix yet (PR criteo#120 is still pending review),
so any branch cut from master right now hits the same
sqlite3.OperationalError: database is locked INTERNAL ERROR in CI.
Carrying the fix here too so this PR's CI is green independently.
Trivial to reconcile on rebase once criteo#120 merges.
@mbrulatout
mbrulatout merged commit 6b705e1 into criteo:master Jul 3, 2026
5 checks passed
@mbrulatout
mbrulatout deleted the phase2-config-discovery-snapshot branch July 3, 2026 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants